Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Export returns module #198

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Export returns module #198

wants to merge 5 commits into from

Conversation

Git-the-Sanz
Copy link
Contributor

What problem is this solving?

The ability to export all or some return requests into a single downloadable file

How to test it?

Powerplanet
YAMAMAY

Screenshots or example usage:

Screenshot 2022-09-02 at 17 11 36

@Git-the-Sanz Git-the-Sanz requested review from filafb and a user September 2, 2022 15:18
@vtex-io-ci-cd
Copy link

vtex-io-ci-cd bot commented Sep 2, 2022 •
edited by Git-the-Sanz
Loading

Hi! I'm VTEX IO CI/CD Bot and I'll be helping you to publish your app! 🤖

Please select which version do you want to release:

  • Patch (backwards-compatible bug fixes)

  • Minor (backwards-compatible functionality)

  • Major (incompatible API changes)

And then you just need to merge your PR when you are ready! There is no need to create a release commit/tag.

  • No thanks, I would rather do it manually 😞

@vtex-io-docs-bot
Copy link

vtex-io-docs-bot bot commented Sep 2, 2022 •
edited by Git-the-Sanz
Loading

Beep boop 🤖

I noticed you didn't make any changes at the docs/ folder

  • There's nothing new to document 🤔
  • I'll do it later 😞

In order to keep track, I'll create an issue if you decide now is not a good time

  • I just updated 🎉🎉

filafb
filafb requested changes Sep 6, 2022
View reviewed changes
Copy link
Collaborator

@filafb filafb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome job @Git-the-Sanz. Not sure you have discussed that with @cesarocampov, but I think we need to have a solution to bring the refundData information. With the current report, the customers cannot have the whole picture if the request is already approved or refunded. Bringing that information can be complex, but I think it's important to discuss it. Maybe we should have a different report for it.

On another note, I remember that we discussed to bring the refundStatusData in different columns for each status. I see we have a column for new, but it's only bringing NA. Lastly, any idea on how to bring the comments?

node/utils/verifyReportMap.ts Show resolved Hide resolved
react/admin/ExportReturnRequestsModal/components/ReportStatus.tsx Outdated Show resolved Hide resolved
messages/en.json Outdated Show resolved Hide resolved
@Git-the-Sanz
Copy link
Contributor Author

Thanks @filafb! I don't understand what you mean with refundData. Let's talk schedule a meet to talk about it!
Regarding refundStatusData, looks like I never amended it. I'll do it asap.

@filafb
Copy link
Collaborator

filafb commented Sep 6, 2022

Thanks @filafb! I don't understand what you mean with refundData. Let's talk schedule a meet to talk about it! Regarding refundStatusData, looks like I never amended it. I'll do it asap.

The refund data is the object that controls the information regarding what will be refunded for the store user. It has the invoice information, the values that will be refunded but also the items that were approved. That is why it can be complicated to bring the info in the same report. But let's chat about it.

And maybe it's better to wait this chat before working on the refundStatusData. I am seeing a scenario that might be confusing for the final user.

@Git-the-Sanz Git-the-Sanz self-assigned this Sep 7, 2022
@Git-the-Sanz Git-the-Sanz added the enhancement New feature or request label Sep 7, 2022
@Git-the-Sanz Git-the-Sanz force-pushed the feature/admin-export-returns branch from db630a5 to cd4d746 Compare October 6, 2022 13:23
@Git-the-Sanz Git-the-Sanz force-pushed the feature/admin-export-returns branch from cd4d746 to 82f2842 Compare January 26, 2023 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@filafb filafb filafb requested changes

Assignees

@Git-the-Sanz Git-the-Sanz

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Git-the-Sanz @filafb